Skip to content

Allow multiple success responses in ApolloCall.execute() #6772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Allow multiple success responses in ApolloCall.execute() #6772

wants to merge 1 commit into from
+3 −4

Conversation

@BoD
Copy link
Contributor

@BoD BoD commented Nov 4, 2025 •
edited
Loading

Rationale / decision record

  • Calling execute() on subscriptions or other multiple success responses cases makes little sense, which is why we were failing fast with a clear message
  • However the cache's CacheAndNetwork fetch policy can emit 0, 1, or 2 success responses, so execute() may not always crash - e.g. it doesn't crash during tests, but crashes in production (aka, "foot gun").
    • User facing this: Slack
  • Recognizing this, in Cache v1.0.0-alpha.5, CacheAndNetwork has been deprecated, noting that it is relatively trivial to re-implement it in user code
  • However since projects are already using it, this raised concerns
  • Decision:
    • Don't crash in execute() but rather return the first success response.
    • Revert CacheAndNetwork deprecation

That would be technically a behavior change, but one that can only have 'good' consequences?

Pro/cons

  • Pro: fewer crashes
  • Con: execute() with multiple success responses still make little sense, but by not crashing we're not educating the user

@BoD BoD requested a review from martinbonnin as a code owner November 4, 2025 17:36
@apollo-librarian
Copy link

apollo-librarian bot commented Nov 4, 2025 •
edited
Loading

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 1 changed, 0 removed 
* (developer-tools)/kotlin/v5/advanced/network-connectivity.mdx

Build ID: 0b8b734c2f2892efb6ad9b4e
Build Logs: View logs

URL: https://www.apollographql.com/docs/deploy-preview/0b8b734c2f2892efb6ad9b4e

@martinbonnin
Copy link
Contributor

martinbonnin commented Nov 4, 2025

Can we make a decision record here? Like why we deprecated it, issues where users complained, why we are undeprecating it and now why we are not throwing any more?

@BoD
Copy link
Contributor Author

BoD commented Nov 4, 2025

I've updated the description.

martinbonnin
martinbonnin approved these changes Nov 4, 2025
View reviewed changes
Copy link
Contributor

@martinbonnin martinbonnin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for providing the context. In a way, CacheAndNetwork is "stale-while-revalidate" when used with execute().

@BoD BoD mentioned this pull request Nov 5, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martinbonnin martinbonnin martinbonnin approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@BoD @martinbonnin